fix(apigateway): set authorization scope when authorization type is Cognito by GavinZZ · Pull Request #30035 · aws/aws-cdk

GavinZZ · 2024-05-01T20:08:13Z

Issue # (if applicable)

Reason for this change

Authorization scope is set even when the auth type is None. This will cause deployment failure

12:52:11 PM | CREATE_FAILED | AWS::ApiGateway::Method | ActionsApiGatewayI...oxyOPTIONS041B022A
Resource handler returned message: "Invalid Method authorization type specified. Authorization Scopes are only valid for COGNITO_USER_POOLS authorization type (Servic
e: ApiGateway, Status Code: 400, Request ID: f9c6357b-428e-42a8-884c-07b77939d165)" (RequestToken: bb8de2e9-37b7-ca15-9bd8-547bc7eea134, HandlerErrorCode: InvalidRequ
est)

Description of changes

Check when auth type is not Cognito, set auth scope to none. Not a breaking change because original templates cannot deploy.

Description of how you validated changes

All existing and new tests pass.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

packages/aws-cdk-lib/aws-apigateway/lib/method.ts

aaythapa

Generally lgtm, just some clarifying questions

packages/aws-cdk-lib/aws-apigateway/test/method.test.ts

packages/aws-cdk-lib/aws-apigateway/lib/method.ts

packages/aws-cdk-lib/aws-apigateway/test/method.test.ts

aaythapa

🚢

mergify · 2024-05-10T20:57:40Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2024-05-10T21:26:46Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 0ba8ff9
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2024-05-10T21:27:06Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

viktorvoltaire · 2024-05-20T12:16:22Z

This wiped our scopes, even though we have const authorizer = new CognitoUserPoolsAuthorizer(.... having defined our default options like below wasn't a problem until this was merged. Its also a optional parameter that says

The authorizer will take care of setting the correct authorization type.

        defaultMethodOptions: {
          authorizer: authorizer,
          authorizationScopes: [
            "...."
          ],

This warning log was swallowed by CI/CD but still deployed

[Warning at ...] 'AuthorizationScopes' can only be set when 'AuthorizationType' sets 'COGNITO_USER_POOLS'. Default to ignore the values set in 'AuthorizationScopes'. [ack: @aws-cdk/aws-apigateway:invalidAuthScope]

alfie-bends · 2024-05-31T23:50:06Z

packages/aws-cdk-lib/aws-apigateway/lib/method.ts

+    // When AuthorizationType is None, there shouldn't be any AuthorizationScope since AuthorizationScope should only
+    // be applied to COGNITO_USER_POOLS AuthorizationType.
+    const defaultScopes = options.authorizationScopes ?? defaultMethodOptions.authorizationScopes;
+    const authorizationScopes = authorizationTypeOption === AuthorizationType.COGNITO ? defaultScopes : undefined;


By utilizing authorizationTypeOption instead of authorizationType, you are now forcing people to implement an optional parameter for something was previously set by the authorizer, that was previously recommended to not be set explicitly. It is explicitly stated in the documentation, that the authorizer will set the authorizationType, see line 30 of this file. This is a breaking change.

Yes, this just broke our prod environment after a dependabot update - it should be marked as a breaking change 🙁

aws-cdk-automation · 2024-07-25T21:11:46Z

Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one.

aws-cdk-automation requested a review from a team May 1, 2024 20:08

github-actions bot added bug This issue is a bug. effort/medium Medium work item – several days of effort p1 labels May 1, 2024

mergify bot added the contribution/core This is a PR that came from AWS. label May 1, 2024

GavinZZ added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label May 1, 2024

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label May 1, 2024

GavinZZ marked this pull request as ready for review May 1, 2024 21:10

GavinZZ added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label May 1, 2024

nmussy reviewed May 3, 2024

View reviewed changes

packages/aws-cdk-lib/aws-apigateway/lib/method.ts Show resolved Hide resolved

github-actions bot mentioned this pull request May 6, 2024

Weekly PR metrics report #30073

Closed

aaythapa reviewed May 10, 2024

View reviewed changes

fix(apigateway): set authorization scope when authorization type is None

46ab0e9

GavinZZ force-pushed the yuanhaoz/api_auth_scope_fix branch from 831322b to 46ab0e9 Compare May 10, 2024 18:42

aaythapa approved these changes May 10, 2024

View reviewed changes

Merge branch 'main' into yuanhaoz/api_auth_scope_fix

0ba8ff9

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label May 10, 2024

mergify bot merged commit 38a2284 into main May 10, 2024

mergify bot deleted the yuanhaoz/api_auth_scope_fix branch May 10, 2024 21:27

alfie-bends reviewed May 31, 2024

View reviewed changes

github-actions bot mentioned this pull request Jun 1, 2024

Monthly PR metrics report #30414

Closed

tengell mentioned this pull request Jun 6, 2024

[Snyk] Upgrade aws-cdk-lib from 2.141.0 to 2.142.0 capralifecycle/liflig-cdk#267

Closed

NOUIY mentioned this pull request Jun 7, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#102

Open

niravparikh05 mentioned this pull request Jun 13, 2024

[Snyk] Upgrade aws-cdk from 2.65.0 to 2.142.1 paralus/eks-blueprints-addon#3

Open

NOUIY mentioned this pull request Jun 14, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#103

Open

snyk-io bot mentioned this pull request Jun 20, 2024

[Snyk] Upgrade aws-cdk-lib from 2.138.0 to 2.143.1 Opetushallitus/heratepalvelu#280

Closed

NOUIY mentioned this pull request Jun 20, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#104

Open

snyk-io bot mentioned this pull request Jun 21, 2024

[Snyk] Upgrade aws-cdk-lib from 2.139.0 to 2.143.1 Opetushallitus/ovara#36

Open

This was referenced Jun 22, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#105

Open

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#106

Open

NOUIY mentioned this pull request Jul 5, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#107

Open

moelasmar mentioned this pull request Jul 10, 2024

❗ NOTICE (aws-apigateway): Authorization Scopes not rendered with CognitoUserPoolsAuthorizer #30444

Closed

This was referenced Jul 12, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#108

Open

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#109

Open

NOUIY mentioned this pull request Jul 19, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#110

Open

aws locked as resolved and limited conversation to collaborators Jul 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(apigateway): set authorization scope when authorization type is Cognito#30035

fix(apigateway): set authorization scope when authorization type is Cognito#30035
mergify[bot] merged 2 commits intomainfrom
yuanhaoz/api_auth_scope_fix

GavinZZ commented May 1, 2024 •

edited

Loading

Uh oh!

Uh oh!

aaythapa left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aaythapa left a comment

Uh oh!

mergify bot commented May 10, 2024

Uh oh!

aws-cdk-automation commented May 10, 2024

Uh oh!

mergify bot commented May 10, 2024

Uh oh!

viktorvoltaire commented May 20, 2024 •

edited

Loading

Uh oh!

alfie-bends May 31, 2024

Uh oh!

tibbing Jun 17, 2024

Uh oh!

aws-cdk-automation commented Jul 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

GavinZZ commented May 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

Uh oh!

Uh oh!

aaythapa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aaythapa left a comment

Choose a reason for hiding this comment

Uh oh!

mergify bot commented May 10, 2024

Uh oh!

aws-cdk-automation commented May 10, 2024

AWS CodeBuild CI Report

Uh oh!

mergify bot commented May 10, 2024

Uh oh!

viktorvoltaire commented May 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alfie-bends May 31, 2024

Choose a reason for hiding this comment

Uh oh!

tibbing Jun 17, 2024

Choose a reason for hiding this comment

Uh oh!

aws-cdk-automation commented Jul 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

GavinZZ commented May 1, 2024 •

edited

Loading

viktorvoltaire commented May 20, 2024 •

edited

Loading